Skip to content

Add SkolemType in metaprogramming #13579

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Conversation

soronpo
Copy link
Contributor

@soronpo soronpo commented Sep 22, 2021

I ran into an error that somehow got a SkolemType during type-dependent metaprogramming.
Because it was not handled, I got a match type error during .show.
Therefore I added SkolemType, similarly to the other types.
A few questions:

  1. Should a public API to access a SkolemType be available, or should we just handle the .show use-case?
  2. For .show I chose to print it as $skolem[info]. Is there a better alternate suggestion?

@soronpo soronpo added area:metaprogramming needs-minor-release This PR cannot be merged until the next minor release labels Sep 22, 2021
@nicolasstucki
Copy link
Contributor

We should not have skolems in this API. TASTY widens all skolems, hence we should do the same. If we see a skolem it is a bug invovling a missing widenSkolem somewere.

@soronpo
Copy link
Contributor Author

soronpo commented Sep 22, 2021

Ok, then what if I remove the public API but leave the printing to avoid a compiler crash or alternatively throw an error with more details on the skolem?

@soronpo soronpo closed this Sep 26, 2021
@nicolasstucki
Copy link
Contributor

A fix for one instance of the skolem issue in available in #13748.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-minor-release This PR cannot be merged until the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants